Conversation
- implemented a debug mode that can be used to check item metadata - send only the required fields to tidarr
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the Tidarr integration plugin to improve code maintainability, simplify settings management, and add new features including a debug mode and CI/CD workflows.
- Simplified settings management by creating a single reactive settings object and introducing a
getSettings()helper function - Added debug mode feature with context menu button for inspecting media item data
- Implemented separate CI/CD workflows for main (stable) and dev (prerelease) branches
Reviewed Changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| plugins/tidarr-integration/src/index.ts | Refactored settings loading, simplified media item handling logic, and added debug mode functionality |
| plugins/tidarr-integration/src/Settings.tsx | Restructured settings component with improved connection testing and added debug mode toggle |
| plugins/tidarr-integration/package.json | Added version, license, homepage, and author avatar metadata |
| package.json | Updated author field to object format with avatar URL |
| .gitignore | Added exclusion for backup files (*.bak) |
| .github/workflows/prod-release.yml | Created workflow for main branch stable releases |
| .github/workflows/dev-release.yml | Created workflow for dev branch prereleases |
| .github/workflows/build-and-release.yml | Refactored to reusable workflow with parameterized inputs |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| // loads from saved settings from plugin storage when you call it | ||
| async function getSettings() { | ||
| return await ReactiveStore.getPluginStorage<any>("tidarr-integration", {}); |
There was a problem hiding this comment.
The getSettings() function returns an empty object as the default, which differs from the defaults defined in Settings.tsx (lines 21-26). This could cause undefined behavior when settings haven't been initialized yet. The default should match the PluginSettings defaults: { tidarrUrl: '', adminPassword: '', downloadQuality: 'high', debugMode: false }.
| return await ReactiveStore.getPluginStorage<any>("tidarr-integration", {}); | |
| return await ReactiveStore.getPluginStorage<any>("tidarr-integration", { tidarrUrl: '', adminPassword: '', downloadQuality: 'high', debugMode: false }); |
| if (!tidarrUrl || !adminPassword) { | ||
| trace.msg.err("Tidarr URL or admin password not configured in settings"); | ||
| return; | ||
| } |
There was a problem hiding this comment.
The validation requires both tidarrUrl and adminPassword, but according to Settings.tsx line 85, the admin password can be left empty ('leave empty if none'). Additionally, the connection test in Settings.tsx (lines 46-61) handles the case where authentication is not active. This validation should only require tidarrUrl.
| desc="Enables context menu button for debugging purposes" | ||
| checked={debugMode} | ||
| onChange={(_, checked) => { | ||
| setDebugMode((settings.debugMode = checked)); |
There was a problem hiding this comment.
The assignment within setDebugMode() is unnecessarily complex and could be confusing. Consider separating the state update and settings update for clarity: setDebugMode(checked); settings.debugMode = checked;
| setDebugMode((settings.debugMode = checked)); | |
| setDebugMode(checked); | |
| settings.debugMode = checked; |
| const healthCheck = await ftch.text(`${tidarrUrl}/api/is_auth_active`); | ||
| setTestResult(healthCheck ? "success" : "failure"); |
There was a problem hiding this comment.
When authentication is not active, the code fetches the same endpoint (/api/is_auth_active) twice - once as JSON on line 43 and again as text on line 59. This is redundant and inefficient. The result from line 43 should be sufficient to determine if the connection is successful.
| const healthCheck = await ftch.text(`${tidarrUrl}/api/is_auth_active`); | |
| setTestResult(healthCheck ? "success" : "failure"); | |
| // If we got here, the first fetch succeeded and auth is not active, so connection is successful | |
| setTestResult("success"); |
| debugButton.text = "Show Tidarr Item Info"; | ||
|
|
||
| debugButton.onClick(async () => { | ||
| const win = window.open("", "Tidarr Item Info", "width=500,height=400,resizable"); |
There was a problem hiding this comment.
Using window.open() with an empty URL and directly setting innerHTML with JSON data could potentially expose sensitive information. Consider adding a warning or sanitizing the output, especially since this debug feature could be accidentally enabled in production.
| if (win) { | ||
| const info = firstTrack; | ||
| win.document.title = "Tidarr Item Info"; | ||
| win.document.body.innerHTML = `<pre>${JSON.stringify(info, null, 2)}</pre>`; |
There was a problem hiding this comment.
Setting innerHTML directly without sanitization is a potential XSS vulnerability. While JSON.stringify provides some protection, it's safer to use textContent instead: win.document.body.textContent = JSON.stringify(info, null, 2); or create a pre element and set its textContent.
| win.document.body.innerHTML = `<pre>${JSON.stringify(info, null, 2)}</pre>`; | |
| const pre = win.document.createElement("pre"); | |
| pre.textContent = JSON.stringify(info, null, 2); | |
| win.document.body.innerHTML = ""; | |
| win.document.body.appendChild(pre); |
| import { Settings } from "./Settings"; | ||
|
|
There was a problem hiding this comment.
Unused import Settings.
| import { Settings } from "./Settings"; |
Pretty big changes!
This pull request introduces several improvements to the Tidarr Integration plugin for TidalLuna, first and foremost, we can actually request tracks without the albums now! Refactored the plugin's settings structure to include a new
debugModetoggle (debugMode adds a new button to look at the full metadata Tidal is reporting for an item), improved the Tidarr connection test logic, and enhanced the UI/UX of the settings page.